Skip to content

Post-merge-review: Fix template-require-lang-attribute: validate every BCP47 subtag from 'language-tags'#2683

Merged
NullVoxPopuli merged 2 commits intoember-cli:masterfrom
johanrd:day_fix/template-require-lang-attribute
Apr 15, 2026
Merged

Post-merge-review: Fix template-require-lang-attribute: validate every BCP47 subtag from 'language-tags'#2683
NullVoxPopuli merged 2 commits intoember-cli:masterfrom
johanrd:day_fix/template-require-lang-attribute

Conversation

@johanrd
Copy link
Copy Markdown
Contributor

@johanrd johanrd commented Apr 13, 2026

Port only validated parts[0] against a small set; upstream validates every hyphen-separated subtag. Merge COMMON_LANG_CODES and COUNTRY_CODES into a single set and match upstream's 'every subtag OR whole tag in table' semantics. Fixes:

  • lang="yue-Hans" correctly accepted (was rejected)
  • lang="en-XX" still rejected
  • 5 mixed-case entries that were unreachable after toLowerCase fixed

Cowritten by claude

Port only validated parts[0] against a small set; upstream validates
every hyphen-separated subtag (region/script). Add the missing
country-codes table and traverse all subtags. lang="en-XX" is now
correctly flagged.
Comment thread lib/utils/country-codes.js Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have to maintain this ourselves? can we use a library and make it someone else's problem?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, maybe language-tags (mattcg) is the right fit? one dep, active, backed by the IANA registry, and it gives us deprecation suggestions for free. We can drop all 628 lines of country-codes.js. Only wrinkle: v2
▎ requires Node ≥22 and this repo's engines.node is >= 20.19,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a prior version that support 20?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed it now.

language-tags rejects deprecated grandfathered tags (i-klingon → use tlh). The previous validator also rejected those, so no test regression — but consumer templates relying on those tags would now lint-fail. Not in any test today; just worth flagging (claude).

@johanrd johanrd force-pushed the day_fix/template-require-lang-attribute branch from 9964ce4 to 0a7da06 Compare April 13, 2026 17:39
- Drop lib/utils/country-codes.js (628 lines).
- Use language-tags (registry-backed validation via the IANA
  language-subtag-registry transitive dep) instead of a per-subtag
  allowlist; keep the empty/whitespace early-return.
- Pinned to ^1.0.9 (last CJS line; v2.x is ESM).

Behavioral note: language-tags rejects deprecated grandfathered tags
(e.g. `i-klingon`, preferred value `tlh`). The previous hand-rolled
validator also rejected `i-klingon` (its subtags weren't in the table,
nor was the whole tag), so existing tests are unchanged — but consumer
templates relying on deprecated tags would now be flagged.
@johanrd johanrd marked this pull request as ready for review April 13, 2026 17:51
@johanrd johanrd changed the title Post-merge-review: Fix template-require-lang-attribute: validate every BCP47 subtag (single table) Post-merge-review: Fix template-require-lang-attribute: validate every BCP47 subtag (single table) from 'language-tags' Apr 14, 2026
@johanrd johanrd changed the title Post-merge-review: Fix template-require-lang-attribute: validate every BCP47 subtag (single table) from 'language-tags' Post-merge-review: Fix template-require-lang-attribute: validate every BCP47 subtag from 'language-tags' Apr 14, 2026
@johanrd johanrd requested a review from NullVoxPopuli April 15, 2026 13:47
@NullVoxPopuli NullVoxPopuli merged commit fd67dd9 into ember-cli:master Apr 15, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants